Don't set null on M2M fields #28
Conversation
| # check for M2M fields setting if we set null to True. Prevent | ||
| # those warnings by setting null only for non-M2M fields. | ||
| if django.VERSION < (1, 8) or not field.many_to_many: | ||
| field.null = True |
There was a problem hiding this comment.
Setting many to many fields null had no effect before Django 1.8, correct? Maybe just make this if not field.many_to_many:
There was a problem hiding this comment.
Setting many to many fields null had no effect before Django 1.8, correct?
That is correct and this PR leaves the Django < 1.8 behavior the same. The reason for the Django version test is that many_to_many is an attribute that was added to django.db.models.fields.Field in Django 1.8. I'm hoping Django 1.7 is not long for this world so hopefully this can merge as is and then the Django version conditional can be removed when 1.7 support is dropped.
|
@craigds Any thoughts on this change? |
|
sorry. looks good. I think the addition to the docs is probably unnecessary, since null=True makes no sense for M2M fields anyway. |
Sounds good to me, reverted the doc change in 6a6c750 |
Don't set null on M2M fields. Thanks @naegelyd and @lucaswiman
|
Thanks for the review and merge @craigds! |
|
thank you :) |
This PR changes the behavior of
django-typed-modelsso that thenullattribute is not set on M2M fields. Currently,django-typed-modelswants to setnull=Truefor all fields. Looking at the README, we see this is marked as a limitation:Django, however, recognizes that setting
null=Truehas no effect on M2M fields. When an M2M field hasnull=True, the Django system check will generate warnings of the form:To avoid these warnings in applications that use
django-typed-model, this PR updates the original behavior to ignore M2M fields. For all non-M2M fields,nullis still set to true. If, however, a field is M2M, thenullattribute is left alone.